fix(web-integration): prevent dropped characters when input re-renders mid-type#2518
fix(web-integration): prevent dropped characters when input re-renders mid-type#2518quanru wants to merge 5 commits into
Conversation
Deploying midscene with
|
| Latest commit: |
15a1239
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://7e7f89d4.midscene.pages.dev |
| Branch Preview URL: | https://fix-web-clear-then-type-race.midscene.pages.dev |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3945686707
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| obs.observe(document.body, { | ||
| childList: true, | ||
| subtree: true, | ||
| attributes: true, | ||
| characterData: true, |
There was a problem hiding this comment.
waitForDomQuiet observes document.body with subtree/attributes/characterData, so any unrelated mutations anywhere on the page keep resetting the quiet timer until the 500ms hard timeout. Because typeText now calls this on every replace-mode input (web-page.ts), dynamic pages with ticking counters/spinners can incur an extra ~500ms per field and may hit higher-level action timeouts even when the target input is stable. Limit observation to the target element (or a narrow ancestor) so unrelated DOM churn does not throttle typing.
Useful? React with 👍 / 👎.
quanru
force-pushed
the
fix/web-clear-then-type-race
branch
from
1d4fd7f to
11b69f3
Compare
quanru
force-pushed
the
fix/web-clear-then-type-race
branch
from
af20059 to
c86b89d
Compare
…s mid-type
In `replace` mode, `typeText` calls `clearInput` and then immediately
runs `keyboard.type`. `clearInput` fires a synthetic `input` event with
`value === ''`, which many controlled components react to by
re-rendering and replacing the input element. If that replacement lands
AFTER `clearInput` returns but DURING `keyboard.type`, the first
characters land on the now-detached old element and are lost.
Add `Page.waitForDomQuiet({ quietMs, timeoutMs })` that uses a
`MutationObserver` to wait until the DOM has been mutation-free for
`quietMs` (default 100ms), bounded by `timeoutMs` (default 500ms).
Call it in the replace branch of `typeText` between `clearInput` and
`keyboard.type` so async re-renders are absorbed before typing starts.
Adds a deterministic puppeteer-driven regression test that simulates
the re-render pattern and demonstrates the bug on the raw path while
verifying the fix on the production `Input` action path.
…tion The initial `settleTimer = setTimeout(done, q)` caused the MutationObserver to disconnect after `quietMs` even when nothing had mutated yet. Frameworks that re-render later than `quietMs` (e.g. via `setTimeout` or async effects) were missed, so the typed characters still landed on the about-to-be-replaced element. Drop the initial settleTimer: only resolve on (a) `quietMs` of stillness after at least one observed mutation, or (b) the `timeoutMs` cap. The no-mutation path now waits the full `timeoutMs` before returning, which is the unavoidable cost of catching late re-renders.
quanru
force-pushed
the
fix/web-clear-then-type-race
branch
from
c86b89d to
650cc3f
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 650cc3f307
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| attributes: true, | ||
| characterData: true, | ||
| }); | ||
| const hardTimer = setTimeout(done, total); |
There was a problem hiding this comment.
Separate from the earlier subtree-scope comment, the fresh issue here is that waitForDomQuiet only schedules settleTimer from the MutationObserver callback; when a replace-mode Input clears a normal/static field and no DOM mutation follows, the promise cannot resolve after quietMs and always waits for the 500 ms hard timeout. Because this path now runs for every default input replacement, filling otherwise stable forms gets an avoidable 500 ms delay per field and can push higher-level actions over their timeout; start the settle timer immediately after observe so the no-mutation case resolves after quietMs.
Useful? React with 👍 / 👎.
ottomao
left a comment
ottomao
left a comment
There was a problem hiding this comment.
Could we just use a hard sleep ?
|
@ottomao Thanks, I checked the hard-sleep option. I do not think a fixed 300ms sleep is sufficient for this race. In the reproduced case, the input is replaced 450ms after the clear-triggered I kept the DOM-quiet wait, but narrowed its scope in I also added coverage for the scoped behavior in Validation:
|
2 participants
Summary
typeTextin replace mode (the default for theInputaction) callsclearInputand then immediately runskeyboard.type.clearInputfires a syntheticinputevent withvalue === "", which many controlled components react to by re-rendering and sometimes replacing the input element entirely. If that replacement lands afterclearInputreturns but duringkeyboard.type, characters can land on the detached old element and be dropped.Fix
Page.waitForDomQuiet({ quietMs, timeoutMs, target })topackages/web-integration/src/puppeteer/base-page.ts. It uses aMutationObserverscoped to the target input's stable ancestor (closest("form"), then parent element, thendocument.bodyas fallback), so unrelated page-level mutations do not keep resetting the quiet timer.waitForDomQuietas an optional method onAbstractWebPageso shared input code can call it safely.createWebInputPrimitivesreplace mode, callawait page.waitForDomQuiet?.({ target })betweenclearInputandkeyboard.type. Async re-renders triggered by clearing are now absorbed before typing starts.Regression test
packages/web-integration/tests/unit-test/puppeteer/input-clear-then-type-race.test.tsdrives a real Puppeteer browser against a page that replaces the input element after the clear-triggeredinputevent. The productionInputaction path now preserves the complete value (Hello).The same test file also covers the scoped observer behavior: unrelated mutations outside the target ancestor no longer force the wait to run until the hard timeout.
Test plan
pnpm run lintnpx nx test @midscene/web -- input-clear-then-type-race.test.ts